Skip to content
This repository has been archived by the owner on Feb 4, 2025. It is now read-only.

Add a first JSON macro based on package:json/json.dart. #68

Merged
merged 4 commits into from
Sep 27, 2024

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Sep 12, 2024

Adds a minimal JSON macro, lots of TODOs.

Adds return types for fields to the model + (analyzer only) query handling.

Skip golden tests for analyzer/CFE if there are no goldens, instead of failing.

Uses templating for identifiers as suggested in dart-lang/language#1989 (comment) ... this is just a proof of concept, we should discuss whether we want templating and if so how it fits into the overall design: it could be client side only or we make the template format part of the protocol. (In the sense of "this string has this specified format which won't change, if you want something different add a new type / field").

I didn't look too closely at the augmentation golden file, just make it approximately match the package:json/json.dart output for the same code. I realize we need yet another type of test, to compile the code with macro applications and then test with it, which in this case would test the JSON produced by the JSON macro. I'll look at how to add that...

@davidmorgan davidmorgan force-pushed the json-macro branch 3 times, most recently from 379819c to f335ef9 Compare September 12, 2024 09:47
@davidmorgan davidmorgan marked this pull request as ready for review September 12, 2024 10:22
@davidmorgan
Copy link
Contributor Author

Filed an issue #70 for the templating discussion, with some initial thoughts on the topic :)

pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Outdated Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Outdated Show resolved Hide resolved
Copy link
Contributor Author

@davidmorgan davidmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I realize I added a lot of TODOs rather than making changes in this PR, hopefully that's not too annoying, they all seem fairly big and about equal priority so splitting to many PRs seems like a good way to keep things moving.

@@ -11,6 +11,16 @@
"isField": true,
"isMethod": false,
"isStatic": false
},
"returnType": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely: as we discussed we probably want to end up with something flat but probably still giving basic "available properties" at each point.

Added a TODO in definitions.dart

// TODO(davidmorgan): make Member a union.

pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Outdated Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Show resolved Hide resolved
pkgs/_test_macros/lib/json_codable.dart Outdated Show resolved Hide resolved
@amrgetment
Copy link

Congratulations 🙌

@davidmorgan davidmorgan merged commit 98bd197 into dart-archive:main Sep 27, 2024
43 checks passed
@davidmorgan davidmorgan deleted the json-macro branch September 27, 2024 10:19
@davidmorgan davidmorgan mentioned this pull request Oct 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants